Skip to content

feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers - #16887

Draft
os-musk wants to merge 9 commits into
mainfrom
claude/issue-16318-numeric-column-representation-table
Draft

feat(spec,driver-sql,cli): one physical representation for the NUMERIC column family, read by all three producers#16887
os-musk wants to merge 9 commits into
mainfrom
claude/issue-16318-numeric-column-representation-table

Conversation

@os-musk

@os-musk os-musk commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16318

Clause-②: yes

Ruled C ∩ ④ (director seat, decision batch 86, 2026-09-08). One explicit per-field-type
physical-representation table lives in packages/spec, and all three producers read it:
SqlDriver.createColumn, os generate migration --format sql, and the typescript format.
New tables only — no existing column is retyped, no migration is planned, no backfill runs.

The two measurements the ruling named as pre-work

Both were taken before any number was chosen. Every reading below carries a firing control in
the same run, and every count was read from the harness's own printed count, never from a pipe.
Every reading is pinned to the PR head c0d4ed6f9f.

1. The three-producer table, re-run on PostgreSQL 16.13

One object, seven plain numeric declarations, three producers (initObjects; --format sql via
db.raw; the typescript format imported and its up(db) called), read back out of
information_schema.columns — with numeric_precision / numeric_scale, which is the half the
original report's bare data_type read hid.

BEFORE (c930f8597, the branch point):

column        driver                sql gen               ts gen
f_number      real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_currency    real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_percent     real                  numeric(5,2)          numeric(8,2)     DIVERGE
f_slider      real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_summary     real                  numeric(18,2)         numeric(8,2)     DIVERGE
f_progress    real                  numeric(5,2)          numeric(8,2)     DIVERGE
f_rating      real                  integer               integer          DIVERGE
COUNT NUMERIC: diverge=7 agree=0 probed=7

CONTROL (the #16091 character family + boolean + date — expect all AGREE)
f_text        text                  text                  text             AGREE
f_email       character varying     character varying     character varying AGREE
f_boolean     boolean               boolean               boolean          AGREE
f_date        date                  date                  date             AGREE
COUNT CONTROL: diverge=0 agree=4 probed=4

AFTER, same command, same database:

f_number      numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_currency    numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_percent     numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_slider      numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_summary     numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_progress    numeric(65,30)        numeric(65,30)        numeric(65,30)   AGREE
f_rating      integer               integer               integer          AGREE
COUNT NUMERIC: diverge=0 agree=7 probed=7   |   COUNT CONTROL: diverge=0 agree=4 probed=4

⭐ The divergence is wider than the card reported: six of the seven were a THREE-way split,
not a two-way one. table.decimal(name) with no arguments is knex's decimal(8, 2), so the two
halves of one command never agreed with each other either. The card's data_type read could not
see it.

2. The DECIMAL(5,2) truncation — measured, and the inference is CORRECTED

The order is explicit that this was an inference off the DDL literal and that a rounding or a
validating generator path re-opens the whole weighting. Executed on PostgreSQL 16.13:

value                                      numeric(5,2)   numeric(18,2)  real     numeric(65,30)
33.333   (whole-percent storage)           33.33 ALTERED  33.33 ALTERED  EXACT    EXACT
0.33333  (FRACTION storage — the default)  0.33  ALTERED  0.33  ALTERED  EXACT    EXACT
33.336   (rounds UP if it rounds at all)   33.34 ALTERED  33.34 ALTERED  EXACT    EXACT
12.34    (FIRING CONTROL — fits all)       EXACT          EXACT          EXACT    EXACT
COUNT control_exact=5/5

⚠️ The direction named was wrong and the substance holds. PostgreSQL rounds half-up
(33.336 arrives as 33.34); it does not truncate. The loss is silent either way.

And nothing upstream prevents it — this is the half that decides whether the weighting re-opens.
Driven through validateRecord itself:

percent,  NO declared scale   <- 0.33333        ACCEPTED   value UNCHANGED
percent,  max:100, no scale   <- 33.333         ACCEPTED   value UNCHANGED
currency, NO declared scale   <- 1234567.89     ACCEPTED   value UNCHANGED
number,   NO declared scale   <- 33.333         ACCEPTED   value UNCHANGED
CONTROL percent, scale: 2     <- 0.33333        REFUSED code=VALIDATION_FAILED
CONTROL number,  scale: 2     <- 33.333         REFUSED code=VALIDATION_FAILED
COUNT undeclared_accepted=4/4  control_refused=2/2  any_value_mutated=0/6

The generator path neither rounds nor validates. For a field that declares no scale the
COLUMN is the only thing deciding, and a narrow one silently alters data. The weighting that made
option A insufficient stands; only the word "truncate" was wrong. record-validator.ts states the
platform's own position on exactly this (#7501, maintainer ruling 2026-08-11): an over-scale value
"is refused the way an out-of-range one is; silent rounding is silently altering data".

⚠️ summary has NO seam at all — it is platform-computed and validateRecord's type door
excludes it — so for that member the column is the only guard there has ever been.

Where 65,30 comes from — both numbers are dialect maxima, not taste

Nine-value corpus, live PostgreSQL 16.13, written and read back through the driver's own pg type
parsing. Control: 12.5, a dyadic rational every candidate holds exactly — 0 of 5 lost it.

   real            3/9 altered   <- the driver today
   numeric(8,2)    9/9 altered   <- the typescript format today
   numeric(18,2)   7/9 altered   <- the sql format today
   numeric(38,17)  2/9 altered
   numeric(65,30)  0/9 altered   <- this table

real's three are the ones that matter: 1234567.89 reads back 1234567.9 and
Number.MAX_SAFE_INTEGER reads back 9007199000000000. That is the money-fidelity defect the
card named, as a reading rather than an argument.

65 and 30 are MySQL's documented DECIMAL maxima — the binding constraint among the dialects
this platform speaks (PostgreSQL's ceiling is 1000 digits, SQLite has none). ⚠️ An unconstrained
numeric is NOT portable: measured through knex's compilers, decimal(name, null) compiles to
decimal on pg, to float on better-sqlite3, and THROWS on mysql2 ("Specifying no
precision on decimal columns is not supported"). A stated pair is the only spelling all three
accept, which is what the ruling asked for.

The residual bound, stated rather than assumed. An exact decimal is bounded where a float is
not: magnitudes below 1e-30 round to zero and magnitudes at or above 1e35 are REFUSED, where
real kept about seven significant digits out to ~1e38. A refusal is loud and the rounding it
replaces was not, and the sql format's numeric(18,2) already refuses everything at or above
1e16 today.

THE FOSSILS, quoted

SqlDriver.createColumn's float arm — the fossil for the SQLite affinity choice:

rating/slider/progress are authored as numeric scalars (a star count, a slider position,
a percent-of-completion). Without an explicit case they fell to default → table.string,
giving the column TEXT affinity so SQLite coerced the written number to a string ('4' not 4) —
a silent type-fidelity leak the value-loss tests didn't catch. REAL affinity round-trips them
as JS numbers (#field-zoo).

The ADR-0113 reasoning at the end of createColumn, for the nullability half:

ADR-0113: the physical NOT NULL comes from the EXPLICIT storage constraint, not from
requiredrequired is the write-time contract enforced by the record validator at the
engine seam, and binding the DDL to it made every post-deploy tightening a destructive
migration.

Neither fossil contradicts these instructions. Both are carried into the new arms verbatim rather
than summarised away.

SQLite, per type — the constraint the card raised, answered

knex 3.3.0 / better-sqlite3, compiled DDL and live storage class:

   table.float(c)           -> float          real:4      real:4.5  real:33.333  real:0.33333
   table.decimal(c, 65, 30) -> float          real:4      real:4.5  real:33.333  real:0.33333
   table.integer(c)         -> integer        integer:4   real:4.5  real:33.333  real:0.33333
   table.string(c)          -> varchar(255)   text:4.0    <- the fossil's own leak, still there
COUNT identical_float_vs_decimal=1/1   COUNT text_affinity_leaks_outside_string_column=0

Per type, for every type moved out of the float arm:

  • number / currency / percent / slider / progress / summary — BYTE-IDENTICAL
    SQLite DDL to the float arm they leave. ColumnCompiler_SQLite3.prototype.decimal is the
    literal 'float', the same string floating resolves to. They keep REAL affinity, the fossil's
    leak stays defeated, and SQLite applies no precision and no scale — so the exactness this table
    buys is a PostgreSQL/MySQL property and SQLite behaves exactly as it does today.
  • rating — moves to INTEGER affinity. 4 is stored as the integer 4 rather than the real
    4.0, and 4.5 is still accepted as a REAL: SQLite refuses no fractional value, so nothing
    this dialect accepts today stops being accepted. The refusal rating gains is
    PostgreSQL/MySQL-only.

The table.string control in the same run still compiled to varchar(255) and still stored
text:4.0, so "identical" above is a discriminating reading and not a constant.

⭐ A blocking consequence nobody had measured: the READ path

Moving the driver's numeric columns to an exact decimal changes the JS type they read back as.
Measured on live PostgreSQL 16.13:

  c_real   typeof=number   value=12.5
  c_num    typeof=string   value="12.500000000000000000000000000000"
  c_int    typeof=number   value=4
  c_dp     typeof=number   value=12.5
COUNT numeric_is_string=1/1  real_is_number=1/1  (control: `real` still reports `number`)

node-postgres parses numeric to a STRING, and mysql2 does the same for DECIMAL. Driven
end-to-end through driver.create / driver.find, the first implementation returned 3 of 4
numeric fields as strings — a wire-contract break, since valueSchemaFor gives the whole class
z.number().finite().

formatOutput's numericFields pass already exists and already repairs exactly this, but it sat
inside the SQLite-only arm, on a premise stated in readPresentationKind's docblock: "The numeric
repair stays SQLite-only: it exists for legacy TEXT-affinity columns, which no other dialect has."
This change falsified that premise, so the pass now runs on every dialect and the fossil
sentence is corrected in place rather than left standing. After that, end-to-end:

  f_number    wrote 0.1234567890123456   read 0.1234567890123456   typeof=number   OK
  f_currency  wrote 1234567.89           read 1234567.89           typeof=number   OK
  f_percent   wrote 0.33333              read 0.33333              typeof=number   OK
  f_rating    wrote 4                    read 4                    typeof=number   OK
COUNT wrong=0/4

1234567.89 is the discriminating value: on the real column it read back 1234567.9.

The nullability half — landed HERE, not in a paired PR

Stated as the order requires. It lands in this PR because both generators' emitters are edited a
few lines apart from the numeric arms, and splitting them would put two halves of one file's
rewrite in two reviews. Both formats now take the physical NOT NULL from storage.notNull and
never from required. Driven on live PostgreSQL 16.13, four declaration shapes, three producers:

column            driver    sql gen   ts gen
f_required_only   YES       YES       YES   AGREE
f_storage_only    NO        NO        NO    AGREE
f_both            NO        NO        NO    AGREE
f_neither         YES       YES       YES   AGREE
COUNT nullability_diverge=0/4
CONTROL: the probe distinguishes its two verdicts (f_storage_only=NO, f_neither=YES)

This unblocks #16294 cause 1. That card's other two causes are not addressed here and stay
on it.

The "new tables only" bound, verified rather than asserted

A table built exactly as the pre-change driver built it (real numeric columns, rows in it) was
put in front of detectManagedDrift alongside a control table carrying the stale-textual shape
the detector DOES report:

COUNT legacy_findings=1  control_findings=2  total=3
  LEGACY   type_mismatch  name    (the pre-existing varchar(255) under a `text` field — unrelated)
  CONTROL  type_mismatch  f_tags  (multi-value field over a varchar column — the #11535 shape)
  CONTROL  type_mismatch  name    (the same unrelated varchar finding)
COUNT numeric_column_findings=0

0 drift findings for the four pre-existing real numeric columns, while the control fired
twice in the same run. Nothing here retypes an existing column, plans a migration, or starts
reporting drift over the difference — the additive sync only ever ADDS columns.

⚠️ The consequence that follows, stated because a reviewer will ask: a table that gains a NEW
numeric field after this lands carries a numeric(65,30) column beside its older real ones.
That is what "new columns only" means, and it is the shape the ruling chose — the alternative is
the migration of existing data it explicitly declined (「不考虑现有数据」).

Gaps that stay OPEN — stated, not silently assumed

  1. The value ranges in existing currency columns are unknown. No deployment was surveyed.
    The triage seat's 「无拉动」 is absence of evidence, not evidence of absence, and nothing here
    changes that.
  2. MySQL was not executed. Every MySQL claim in this PR is knex's compiled DDL plus MySQL's
    documented DECIMAL caps; no MySQL server was driven. PostgreSQL 16.13 and better-sqlite3 were.
  3. SQLite is measured for affinity and storage class, not for scale. SQLite applies neither
    precision nor scale, so "the six members are unchanged there" is a statement about affinity and
    DDL bytes — it is not a claim that SQLite gained exactness.
  4. --format sql remains a PostgreSQL-only claim (os generate migration's audit-stamp columns diverge from driver-sql — the generators emit NOT NULL where the driver emits nullable, and the SQL format emits TIMESTAMP where both knex paths yield timestamptz #15521). Neither generator reproduces the
    driver's dialect branching, and this PR does not change that.

Full file surface

Cross-lane by design — the type table cannot be split across two PRs without producing two
spellings of it.

packages/spec/src/data/numeric-column-representation.ts        NEW  the table + resolver
packages/spec/src/data/numeric-column-representation.test.ts   NEW  the totality pin
packages/spec/src/data/index.ts                                     one export line
packages/spec/api-surface/data.json                                 regenerated
packages/spec/export-origins/data.json                              regenerated
packages/drivers/driver-sql/src/sql-driver.ts                       createColumn + the read pass
packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation.test.ts  NEW
packages/cli/src/commands/generate.ts                               both formats + nullability
packages/cli/src/commands/generate-numeric-column-representation.pin.test.ts     NEW
packages/cli/src/commands/generate-multiple-json-column.pin.test.ts  vehicle updated, see below
.changeset/numeric-column-representation.md                    NEW

⛔ No governed path is touched: nothing under docs/adr/, .claude/, skills/, AGENTS.md,
CLAUDE.md or content/docs/releases/.

⚠️ generate-multiple-json-column.pin.test.ts — the VEHICLE changed, the subject did not. That
pin is about multiple not deciding nullability; required was merely how a NOT NULL was spelled
when it was written. It now spells the constrained case with storage.notNull and asserts the
required-only case is nullable BESIDE it, which is the half that would catch this change being
silently reverted.

#16693 is in flight on adjacent text — left alone

Card #16693 has commits pushed against
packages/spec/src/conversions/registry.ts's field-required-notnull-explicit entry and the
ADR-0113 sentence in sql-driver.ts. ⛔ Neither was edited here. This PR quotes the ADR-0113
sentence and adds a separate #16318 note elsewhere in the file; the conversion registry is not
in this diff at all. If that text moves under this branch, the two land independently.

Verification

Red-before / green-after with the same command, both outputs quoted above (7 of 7 diverge → 0 of
7), plus:

  • Ablation of the new pin. NUMERIC_COLUMN_SCALE mutated 30 → 2 in the spec source, the
    mutation proved on disk (blob hash moved off the HEAD blob; the anchor count moved 1→0 and
    0→1), the pin run RED (exit 1, 1 failed / 4 passed), restored with git checkout HEAD --, the
    restore proved byte-identical to the HEAD blob with an empty git diff HEAD, and the restored
    leg run GREEN (exit 0, 5 passed). Trap-guarded on EXIT/INT/TERM with an absolute path.
  • pnpm --filter @objectstack/driver-sql test — 162 files passed, 10 skipped; 2449 tests passed.
  • pnpm --filter @objectstack/cli exec vitest run --project unit — 186 files, 2561 tests, all
    passing after the pin above was updated (it was RED first, on exactly the assertion this change
    reverses).
  • pnpm --filter @objectstack/cli exec vitest run --project integration on the generator pins —
    36 tests; the new pin is integration-tier (it value-imports the driver) and runs 8 of them.
  • pnpm --filter @objectstack/spec test — 465 files passed, 1 skipped; 12969 tests passed. (The
    new pin is 5 of them.)
  • typecheck for all three packages — exit 0.
  • pnpm --filter @objectstack/spec check:generated — was red on api-surface and
    export-origins, regenerated with the two commands it named, and both are committed.

Gate coverage was derived, not guessed —
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, all 83 run,
then reconciled. The reconciliation line, verbatim:

✓ dispatch-gates --ran: 83 derived famil(ies) accounted for — 83 run, 0 NOT-MEASURED.

⚠️ That line is COVERAGE, not verdicts. The verdicts, stated separately:
81 of 83 exit 0, and 2 exit 3 — NOT MEASURED, never a pass:

  • pnpm check:dual-build-cjs-loadsPREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. It names 12 packages this diff does not touch (studio,
    client-react, the connectors, the plugins). It needs a whole-monorepo pnpm build, which does
    not fit this box's foreground budget; CI builds everything and runs it there. Declared, not
    skipped.
  • pnpm check:i18n-coverage — same cause, one layer out: os lint could not load
    examples/app-showcase/objectstack.config.ts because @objectstack/connector-mcp has no build
    output in this worktree. Its own text says "Nothing was compared… this result says NOTHING".

⚠️ A third gate, pnpm check:type-check-debt, first exited 3 with a V8
FATAL ERROR: Ineffective mark-compacts near heap limit under --max-old-space-size=4096 on this
shared box — its own text says that is "NOT a pass and NOT a finding". Re-measured at 8192 it
exits 0. The pass above is that second run, not the OOM.

The gate list was re-derived against a freshly fetched origin/main after the sweep: identical, 83
families, none added and none dropped, so nothing newly landed upstream is owed.

Level

minor on all three packages — a widening takes at least minor, and major is refused in the
launch window, so breaking-ness rides the BREAKING banner in the changeset plus its ADR-0087
disposition (not-required (no-migration-prescription): nothing an author writes is removed or
renamed, so there is no FROM → TO edit to prescribe). packages/drivers/driver-sql/src/ is a
NESTED package dir, which the level axis cannot see (#16713) — it is graded minor here anyway,
and that grading is the author's, not the gate's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg

The container restart that killed the previous os-dev left this in the
worktree, uncommitted. Committed verbatim so it is not the only copy;
every number in it is re-measured before anything is claimed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
… for the NUMERIC column family

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…e producers

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…(ADR-0113)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…c column table

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation protocol:data tests tooling labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/cli, @objectstack/driver-sql, @objectstack/service-analytics, @objectstack/spec, touching 21 documentable anchor(s). ⚠️ 5 changed file(s) yielded no anchor (packages/services/service-analytics/src/measure-result-type.ts, packages/spec/api-surface/data.json, packages/spec/export-origins/data.json, …), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-flow.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/data-modeling/drivers.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/data-modeling/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/deployment/cli.mdx (via os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/permissions/tenant-audit-census.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/lifecycle.mdx (via SqlDriver (symbol, a top-level class), os generate (command, read off packages/cli/src/commands/generate.ts))
  • content/docs/protocol/objectql/query-syntax.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/objectql/types.mdx (via NUMERIC_COLUMN_REPRESENTATION (symbol, a top-level const object), SqlDriver (symbol, a top-level class), os generate (command, read off packages/cli/src/commands/generate.ts))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via SqlDriver (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 5 changed file(s) yielded no anchor (packages/services/service-analytics/src/measure-result-type.ts, packages/spec/api-surface/data.json, packages/spec/export-origins/data.json, …) — pages documenting those are invisible to this run
  • 9 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 137 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json f7a9740c3c9d78aab77564a26a5027c7542e724fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 779b7fd3c646b1cb5f42695085c8478baf8bf380 — the merge of head 034856799a4107a5c965be47f99ae2cf4d023877 into base f7a9740c3c9d78aab77564a26a5027c7542e724f, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 779b7fd3c646b1cb5f42695085c8478baf8bf380 && git checkout 779b7fd3c646b1cb5f42695085c8478baf8bf380
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f7a9740c3c9d78aab77564a26a5027c7542e724f 034856799a4107a5c965be47f99ae2cf4d023877 && git checkout -B drift-repro f7a9740c3c9d78aab77564a26a5027c7542e724f && git merge --no-ff 034856799a4107a5c965be47f99ae2cf4d023877

node scripts/docs-audit/affected-docs.mjs --json f7a9740c3c9d78aab77564a26a5027c7542e724f

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs f7a9740c3c9d78aab77564a26a5027c7542e724f → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Contributor

Contract review (claude-fable-5-1, isolated seat) — PR #16887 @ c0d4ed6f9

Verdict: CHANGES REQUIRED. The code does what the ruling ordered — one table in packages/spec, all three producers resolve through it, no residual literal, no ALTER, no migration — and the SQLite pins are honest about what SQLite can measure. Two things the PR ships disagree with the repo: a protocol doc that now states the opposite of the new DDL, and a changelog sentence about MySQL that was never executed and that MySQL's documented conversion contradicts, while the live PG+MySQL harness the repo already runs in CI went unused.

Governed-surface check: docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, content/docs/releases/** — none touched (git diff --name-only origin/main...c0d4ed6f9, 11 files, all under packages/** and .changeset/). packages/spec/src/conversions/registry.ts (#16693) not in the diff; the ADR-0113 sentence at the tail of createColumn is quoted, not edited. git merge-tree --write-tree origin/main refs/review/16887 → clean (origin/main = 142c01c88).

Findings

  1. blocking — the protocol doc now states the opposite of the DDL, and was not edited. content/docs/protocol/objectql/types.mdx:266-268 — "SQL driver: a floating-point column (REAL on PostgreSQL/SQLite, FLOAT on MySQL). precision/scale are validation and display metadata — the DDL does not emit NUMERIC(precision, scale)"; :312 the same for currency; the mapping table at :1177 (number/currency/percentREAL/FLOAT/REAL) and :1185 (summaryREAL). content/docs/references/api/sortability.mdx:68 — "summary is an engine-maintained table.float". After this PR a new column is numeric(65,30) on PG/MySQL and rating is integer. The Docs Drift bot on this PR says in its own caveat that types.mdx is exactly the page an emitter-only diff cannot list ("it was the page that diff falsified, in four places" — fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430), so the bot's green is not coverage here. The PR's whole argument is "one declaration, one column"; the protocol doc is now a fourth answer. content/docs/protocol/** is hand-written and ungoverned (AGENTS.md → Documentation Guardrails row for content/docs/<tree>/), so it belongs in this PR, not in a rider. Same fossil, lower stakes: packages/services/service-analytics/src/measure-result-type.ts:161 still records "driver-sql's DDL answers col = table.float(name)" as the reason summary needs no correction.

  2. blocking — the changeset asserts a MySQL refusal that was never executed, and MySQL's documented conversion says the opposite. .changeset/numeric-column-representation.md bullet 1: "rating is an INTEGER column on PostgreSQL and MySQL, so a fractional star count is now REFUSED there". The PR body's own gap 2: "MySQL was not executed … no MySQL server was driven." PostgreSQL refuses '4.5' into integer (text-param input syntax error). MySQL does not: a fractional value assigned to an integer column is rounded (4.5 → 5), with no error in strict mode, and a DECIMAL scale overflow is rounded with a Note (1265). That is a silent alteration on the dialect the changeset names as refusing — the exact class (A number field's declared scale is never enforced — values with more decimals are accepted and stored verbatim (min/max on the same field are enforced) #7501, "silent rounding is silently altering data") the table was chosen to end. The instrument exists in this repo: packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts (+ .globalsetup.ts) keyed on OS_TEST_POSTGRES_URL, used by 16 test files (e.g. sql-driver-aggregation-conformance.test.ts, sql-driver-column-order-dialects.test.ts), and run by the required check "Temporal Conformance (live PG + MySQL)" (.github/workflows/ci.yml:894, PG service + MySQL service conformance). Ask: either measure rating/DECIMAL on MySQL through that harness and pin it, or rewrite the bullet to say PostgreSQL refuses and MySQL rounds. A published changelog cannot carry a dialect claim the author marks unmeasured two sections later.

  3. record — no live pin for the property the PR is about. All three new tests are better-sqlite3 (sql-driver-16318-numeric-representation.test.ts, generate-numeric-column-representation.pin.test.ts, spec pin) and say so honestly ("⛔ Do not read the oracle as a precision check"). The numeric(65,30) DDL, the information_schema three-producer table, the nullability four-shape table and the read-path string→number repair on PG are all PR-body prose from a session, reproducible by nobody from the tree. The env-gated harness in finding 2 is where a numeric_precision/numeric_scale read and a typeof === 'number' read belong; on SQLite the read-path pin at sql-driver-16318-numeric-representation.test.ts:139-149 cannot fire (SQLite never hands back a string for a float column), so the every-dialect move in formatOutput has zero automated coverage on the two dialects it was made for.

  4. record — the read seam is binary64, and that bounds "money fidelity" end-to-end; the every-dialect pass also touches EXISTING columns. sql-driver.ts:17092-17099 does Number(v) on every string in numericFields on every dialect. Measured (node): for a value the driver itself wrote from a JS double the round-trip is exact — '1234567.890000000000000000000000000000'1234567.89, '12.500…'12.5 — because the shortest repr is what was stored. For anything not born as a JS double it is lossy and silent: '1234567890123456.123'1234567890123456; '9007199254740993.000…'9007199254740992; a full 30-scale '0.123456789012345678901234567890'0.12345678901234568. So the column is exact and find() is not: SQL-side writers, summary sums computed in SQL, anything ≥ 2^53. Not new as a wire limit (z.number().finite()), but the PR's "0/9 altered" is a column reading, not a find() reading, and the changeset should say the fidelity gained is exact-column-through-double. Second half: numericFields is seeded from NUMERIC_SCALAR_TYPES (sql-driver.ts:262), which includes the driver aliases integer/int/float for external/introspected columns. node-postgres returns int8/bigint as a string; before this PR that string left formatOutput untouched on PG, after it is Number()-ed — an existing external bigint column under a numeric field type silently rounds above 2^53 on PG. That is a read-path change to existing deployments outside "new tables only", unmeasured and unstated; state it or scope the pass.

  5. record — the residual bound understates its onset. PR/spec docblock: "magnitudes below 1e-30 round to zero". True, but PG rounds any digit past the 30th place silently, so a full-precision double loses trailing significant digits from about |x| < 1e-13 (1.2345678901234567e-15 stores as 0.000000000000001234567890123457). The bound to state is "≤ 30 fractional digits are kept", not "below 1e-30".

  6. record — ADR-0087 disposition rides the detector miss the gate's own docblock warns about. scripts/check-adr-0087-registration.mjs:141-147: no-migration-prescription is refused only when the body carries a prescription, and :199-206 (fix(service-package): classify a publish driver fault as 5xx and stop returning driver text as caller data (#8131) #8277) says an exemption held because the detector found nothing is the failure mode. Changeset bullet 3 is a prescription in substance: "A generated migration no longer emits NOT NULL for a field marked only required: true. Declare storage: { notNull: true } for a physical constraint" — a FROM (required: true) → TO (storage.notNull) edit a consumer must make to keep the column they had, written without the arrow/table framing the detector matches. "Nothing authorable moves" is true of keys; the output of os generate migration for an unchanged input moves (types and NOT NULL), which is what a consumer with checked-in migrations diffs. Either argue the exemption positively in the marker (the ledger is for migrate meta rewrites; this is an addition, not a rename — defensible) or register. Check Changeset is green on all matrix legs, consistent with a miss rather than a finding.

  7. observation — ruling compliance, itemised. (a) One table in packages/specnumeric-column-representation.ts:196-233. (b) Three producers read it ✓ — sql-driver.ts:16483-16489, generate.ts:1134-1136,1212,1243-1245 via numericSqlType, generate.ts:1985-1992; grep of both files at the ref finds no DECIMAL(/decimal(/float( literal for a FieldType: the remaining table.float (:16436) and table.integer (:16429) arms are the driver aliases float/integer/int, which are not in the FieldType enum (field.zod.ts:40-100). (c) New tables only ✓ — the three alterTable sites (:10128,12193,12462) add columns through createColumn; no type alter, no drift reader (schema-drift.ts has no numeric expectation; nullability drift reads storage.notNull, :849). (d) Both pre-work measurements are in the PR body, with controls; the 5,2 inference corrected to rounding. (e) rating integer with the half-star search stated in changeset ✓. (f) Nullability landed here rather than paired — the ruling says "same PR or a paired one", the dispatch says "the dev's call, stated either way"; stated ✓, within the ruling. (g) Deviation worth naming: the ruling asked per-type answers ("precision chosen for money", "a stated scale", "a scale that does not truncate"); the table is per-type in shape and uniform in value — 65,30 for all six, justified as the portable maximum. It satisfies each constraint; it is not what "chosen for money" reads as, and the maintainer should see that it was answered with "the ceiling".

  8. observation — totality and the pin. The pin (numeric-column-representation.test.ts:25-27) enumerates from NUMERIC_VALUE_TYPES, not a second hand list ✓, and NUMERIC_VALUE_TYPES is satisfies readonly FieldType[] (field-value.zod.ts:63-65) — a subset check, itself a hand list. A numeric type added to the FieldType enum but not to that set still falls to default → table.string in the driver; pre-existing, out of this table's reach, worth a sentence. summary belongs in the table (it is stored, was table.float, is in NUMERIC_VALUE_TYPES); "no seam" is true as stated — COMPUTED_VALUE_TYPES excludes it from validateRecord's type door — and the aggregateSummaryValue verbatim min/max over a text child (measure-result-type.ts) is now refused by an exact column exactly as real refused it.

  9. observation — numericSqlType runs at module evaluation. FIELD_TYPE_SQL_MAP is a module const (generate.ts:1134), so a missing entry throws when generate.ts is imported — every os command, not only generate migration. Loud by design; the blast radius is the CLI, and the spec pin fails first in CI.

  10. observation — tests, surface, hygiene. No .skip/.only/.todo in the diff. Ablation: NUMERIC_COLUMN_SCALE 30→2 fails exactly toBe(30) / toBeGreaterThan(2) / >= 5 in one it (:57-64); the other four compare against the constants and stay green — "1 failed / 4 passed" follows. generate-multiple-json-column.pin.test.ts keeps its subject (multiple does not decide nullability) and adds the required-only-is-nullable half. api-surface/data.json / export-origins/data.json: additions only — NUMERIC_COLUMN_PRECISION, NUMERIC_COLUMN_REPRESENTATION, NUMERIC_COLUMN_SCALE (const), NumericColumnRepresentation (type), numericColumnFor (function). Changesets minor ×3 with BREAKING banner; check-changeset-no-major has nothing to refuse; the packages/drivers/driver-sql/src/ level-axis blind spot ([finding] The changeset LEVEL axis is blind to every NESTED package: packages/*/src/** matches one segment, so 51 of 74 workspace packages (all drivers/services/adapters) can pair Clause-②: yes with patch and stay green #16713) is declared and the grade is the author's. Fixes #16318 in the body only; seven commits, none carries a closing trailer. CI at c0d4ed6f9: 55 of 56 check runs completed, all success or skipped (Build Core, Test Core 1–6, Temporal Conformance, Dogfood ×4, Type Check ×4, Check Changeset ×6, Governed Surface Queue Guard, Spec property liveness); "Lint & Repo Gates" still in_progress at read time. mergeable_state: blocked — draft, needs:contract-review with no approving review, one required check unfinished.

Maintainer-only merge: yes. This decides what every NEW table gets for money, percent and rating on PostgreSQL and MySQL — numeric(65,30) read through a JS double, integer for stars — under a ruling recorded as the director seat's reading of 「继续决策」 with "the maintainer may overturn on a word"; it is cross-lane (engine + cli), Clause-②: yes, and finding 2 puts a dialect claim in a shipped changelog that the maintainer's own MySQL deployments would contradict. The maintainer's eyes are warranted, and finding 1 and 2 should be fixed before they look.


Generated by Claude Code

Brings in #16890 (f2b5e46), which withdrew the ADR-0087
field-required-notnull-explicit conversion and rewrote the ADR-0113
comment block this branch quotes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018rzQyhLGC5iVs11V3TzRs5
…ve-dialect pins and the read-path bound

The protocol doc stated the opposite of the DDL this branch emits, and three
claims the branch makes had no instrument behind them.

- content/docs/protocol/objectql/types.mdx: `number`, `currency` and `percent`
  said "a floating-point column (REAL / FLOAT)" and the type-conversion matrix
  said `REAL` / `FLOAT` / `REAL` for the family and for `summary`. All of it now
  states the exact-decimal column, the "new columns only" bound, and the SQLite
  and per-dialect `rating` consequences, with `slider` / `progress` / `rating`
  given rows the matrix never carried.
- packages/spec/src/api/sortability.zod.ts (and its generated
  content/docs/references/api/sortability.mdx): the `summary` fossil said
  "an engine-maintained `table.float`".
- packages/services/service-analytics/src/measure-result-type.ts: the same
  fossil, as the reason `summary` needs no correction. The reason is unchanged;
  only the column it names moved.

New live-dialect cell (`sql-driver-16318-numeric-representation-live-dialects.test.ts`):
`numeric_precision` / `numeric_scale` read off the server's own catalog and
compared against the spec table, the `typeof === 'number'` read that SQLite
cannot exercise, and `rating`'s fractional disposition asserted PER DIALECT —
PostgreSQL refuses, MySQL rounds. The PG half was executed against a live
PostgreSQL 16.13; the MySQL half runs in the "Temporal Conformance (live PG +
MySQL)" job.

`formatOutput`'s numeric read coercion is scoped per dialect. It reads the
authorable `NUMERIC_VALUE_TYPES` half on the server dialects and keeps the wider
`NUMERIC_SCALAR_TYPES` set on SQLite, where the legacy TEXT-affinity repair
lives. The aliases `integer` / `int` / `float` are how an external, introspected
column reaches the driver, and node-postgres hands `bigint` back as a string
precisely because it does not fit a JS double: coercing it would round above
2^53 on a table this change never created, outside the "new columns only" bound.

Changeset: `rating`'s two dialects stated separately, the residual bound
restated as "30 fractional digits are kept", the binary64 read seam named, and
the ADR-0087 disposition argued positively instead of resting on the detector
miss the gate's own docblock warns about.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018rzQyhLGC5iVs11V3TzRs5

os-bill commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Director seat adoption record — summon #20, session_01Tep4AYXZvyBA7jsvne5KZV (os-bill), 2026-09-09T06:59Z. The verdict below is adopted verbatim from an isolated contract-review subagent (explicit model = CONTRACT_REVIEW_TIER). Transcript tier check before adoption: every harness-stamped model field in the subagent transcript reads claude-fable-5-1 (95 stamps, no other value). Head re-read at posting time = 034856799a, unchanged since the review. ⛔ This seat takes no release action on this carrier (no ready flip, no auto-merge, no enqueue, no label write): the owning seat (domain:engine / consolidated seat (takeover claim 5594621006)) adopts this verdict verbatim or discards it, and acts per the state machine.


Contract review (CONTRACT_REVIEW_TIER, isolated seat) — PR #16887 @ 034856799a4107a5c965be47f99ae2cf4d023877

Verdict: PASS WITH FINDINGS

Both blocking items from the prior review (5586825380 @ c0d4ed6f9, carried on the card as 5586839197) are discharged on this head, and four of the six items are discharged in full; F5 is discharged in the changeset but not in the spec docblock the changeset points readers at. Nothing found on the head moves a published contract beyond what the prior review approved. Ruling C ∩ ④ (batch #86, 5581958053) is still executed in substance: one table in packages/spec, three producers resolve through it with no residual literal, new tables only, nullability from storage.notNull in both formats. Head = c0d4ed6f9 + merge of origin/main @ ee2cb6b40 (1be05800c7) + one patch commit (034856799a, session session_018rzQyhLGC5iVs11V3TzRs5); increment = 16 files (git diff origin/main...refs/pr-review/16887); delta since the reviewed head = 7 files (git diff 1be05800c7..034856799a).

Owed items from the prior review

  1. F1 (blocking) — docs falsified, unedited → DISCHARGED. content/docs/protocol/objectql/types.mdx:266-281 (number: exact decimal NUMERIC(65,30)/DECIMAL(65,30)/float, "New columns only", the field's own precision/scale still not emitted), :322-335 (currency, with the 0-/3-digit-currency reason for not fixing two decimals), :353-357 (percent), matrix rows :1200-1201 (family + new rating row) and :1209 (summary), footnote :1236-1251 (new-columns-only, SQLite affinity, PG refuses / MySQL rounds). content/docs/references/api/sortability.mdx is AUTO-GENERATED — DO NOT EDIT (its :5), so the head fixed the source packages/spec/src/api/sortability.zod.ts:64-71 and regenerated :68-75 — I compared the "Considered and deliberately NOT members" paragraph of the docblock (comment prefix stripped) with the mdx: byte-identical, so the next gen:docs will not revert it. Fossil packages/services/service-analytics/src/measure-result-type.ts:159-173 corrected (comment-only). No content/docs page on the head still says REAL/FLOAT/table.float for the family except the corrected sortability.mdx:68.
  2. F2 (blocking) — unexecuted MySQL refusal claim → DISCHARGED. .changeset/numeric-column-representation.md:41-46 now states PostgreSQL REFUSES 4.5, MySQL ROUNDS to 5 "with no error", SQLite unchanged. Pinned per dialect in packages/drivers/driver-sql/src/sql-driver-16318-numeric-representation-live-dialects.test.ts:189-208 (PG: 'refused' and 0 rows; MySQL: 'accepted' and f_rating === 5, not.toBe(4.5)). Execution evidence: see F5 below (step-level in CI; the sibling line's independent live MySQL 8.0.46 reading in 5595029333 agrees).
  3. F3 (record) — no live pin → DISCHARGED. New 281-line cell on the existing harness: catalog numeric_precision/numeric_scale compared to numericColumnFor rather than a literal (:124-154, with a non-vacuity check that the catalog answered and that two distinct arms exist), typeof === 'number' read seam (:156-175), the 1234567.89 / 33.333 discriminators (:177-187), rating per dialect (:189-208), and the F4 scope cell (:235-281). It runs under the "Temporal Conformance (live PG + MySQL)" job: .github/workflows/ci.yml:1170-1183 sets both URLs and OS_EXPECT_LIVE_DIALECT_MATRIX: '1' and runs pnpm --filter @objectstack/driver-sql test = vitest run with no custom include/exclude (vitest.config.ts carries only globalSetup); declareDialectCell (live-dialect-matrix.testkit.ts:506-511) routes an unavailable cell (available: !!MYSQL_URL, :313) to declareUnprovisionedCellexpect.fail under the flag (:372-380), so a missing server is red, not a skip.
  4. F4 (record) — read seam binary64 / pass reaches existing columns → DISCHARGED by scoping, not only by statement. sql-driver.ts:4390-4413 new protected numericValueFields (the NUMERIC_VALUE_TYPES half without the integer/int/float aliases); populated on every registration path — shard alias :9669, registerExternalObject :9810/:9823, initObjects registration :9895/:9920 (all three numericFields write sites have a mirror; no orphan); selected per dialect at :17168-17170 (SQLite keeps the wide set for the legacy TEXT-affinity repair). The binary64 bound is stated at :17151-17164 and in the changeset :53-59. Live PG pin :244-280: an external bigint holding 2^53+1 under an alias-typed field reads back as the unrounded string while the numeric(65,30) column in the same row is coerced to 12.5.
  5. F5 (record) — residual bound understated → PARTIALLY DISCHARGED. Changeset :47-52 now says "keeps 30 fractional digits … loss begins around |x| < 1e-13 and is total below 1e-30". The spec docblock packages/spec/src/data/numeric-column-representation.ts:100-106 still says only "Magnitudes below 1e-30 round to zero" — see Finding F1.
  6. F6 (record) — ADR-0087 disposition on a detector miss → DISCHARGED. Marker at changeset :70 argues positively; its load-bearing claim checks out on origin/main: packages/spec/src/conversions/registry.ts:2021-2050 carries the field-required-notnull-explicit withdrawal tombstone (ADR-0087's field-required-notnull-explicit conversion asserts an implication ADR-0113 abolished — the boot calls it a forward conversion, but its output and the source it prescribes disagree at the storage layer #16693, batch Add comprehensive protocol review and Q1-Q4 2026 optimization roadmap #85, "No migration is owed to anyone"). node scripts/check-adr-0087-registration.mjs --base origin/main --head refs/pr-review/16887 → exit 0. The marker's internal pointer is wrong — see Finding F2.

Derived judgments

  • packages/spec/src/data/numeric-column-representation.ts (new): five new exports on @objectstack/spec/dataNUMERIC_COLUMN_PRECISION, NUMERIC_COLUMN_SCALE, NUMERIC_COLUMN_REPRESENTATION (consts), NumericColumnRepresentation (type), numericColumnFor (function); api-surface/data.json and export-origins/data.json are additions only (+5 each). index.ts:222-227 one export *. Public surface widens → Clause-②: yes is right. No new key on any payload, no new error code, no enum member (FieldType untouched; the table is Record<string, …> with membership pinned by numeric-column-representation.test.ts:25-27 in both directions against NUMERIC_VALUE_TYPES).
  • packages/spec/src/api/sortability.zod.ts:64-71: docblock only; no schema moves.
  • packages/drivers/driver-sql/src/sql-driver.ts: behaviour of a published door moves — createColumn numeric arm :16514-16533 resolves the seven NUMERIC_VALUE_TYPES through numericColumnFor (table.integer / table.decimal(name, 65, 30)), the old summary table.float arm is removed (old :16522-16527), the float alias keeps table.float (:16476-16478); formatOutput numeric coercion now runs on every dialect (:17123-17178), which also repairs EXISTING external objects mapped onto numeric/DECIMAL columns (stated in the PR body). New protected member numericValueFields is subclass-visible surface. New tables only: the three alterTable sites still add columns through createColumn; no type alter; schema-drift.ts unchanged in this diff.
  • packages/cli/src/commands/generate.ts: published CLI output moves — FIELD_TYPE_SQL_MAP entries :1134-1136, :1212, :1243-1245 via numericSqlType (:1307-1316, throws at module evaluation on a missing entry: every os command that imports generate.ts, loud by design, the spec pin fails first); declaredNotNull :1330-1332; NOT NULL from storage.notNull in --format sql :1771 and typescript :1888; typescript numeric arm :1979-1994. generate-numeric-column-representation.pin.test.ts derives every expectation from numericColumnFor (:108, :133, :149, :166-167) and asserts the old literals are gone (:138-139).
  • packages/services/service-analytics/src/measure-result-type.ts:159-173: comment-only; no behaviour, no changeset owed.
  • packages/lint/**, packages/metadata-protocol/**, packages/objectql/**: not in the diff (see Finding F3).
  • Beyond-prior-approval check on the delta: numericValueFields narrows the pass the prior review asked to be scoped; everything else in 034856799a is docs, comments, a test file and changeset prose. Nothing moves further than approved.

Semver / changeset

.changeset/numeric-column-representation.md: @objectstack/spec, @objectstack/driver-sql, @objectstack/cli all minor — a widening takes at least minor, major refused in the launch window. node scripts/check-changeset-no-major.mjs --base origin/main --head refs/pr-review/16887 → "introduces no major bump", exit 0 (level axis reported N/A offline for lack of a PR payload; the CI "Check Changeset" run on the head is green and the axis needs at least one moved package graded minor, which three are). BREAKING banner present at :38. ADR-0087 marker present at :70, gate exit 0. The sibling line's "3 → 7 packages" is not owed by any rule: comment-only edits in lint/metadata-protocol/objectql/service-analytics release nothing, and none is graded patch (the level-axis prohibition). Governed paths in the 16-file increment: none (.claude/**, skills/**, docs/adr/**, AGENTS.md, CLAUDE.md, content/docs/releases/** all untouched; the skills/** rows in a two-dot diff are main's, arriving via 1be05800c7).

Boundary flags

  • Cross-lane by design: packages/cli/** (domain:cli) under a domain:engine claim — declared in the dispatch (5584575074) and the PR body.
  • content/docs/protocol/** is hand-written and ungoverned; in-PR is correct. content/docs/references/api/sortability.mdx is generated; edited through its source — the prior review's instruction to edit the .mdx line directly would have been reverted by the next gen:docs, and the head did the right thing.
  • ADR-0087's field-required-notnull-explicit conversion asserts an implication ADR-0113 abolished — the boot calls it a forward conversion, but its output and the source it prescribes disagree at the storage layer #16693's packages/spec/src/conversions/registry.ts is not in the diff; after the merge the ADR-0113 block in sql-driver.ts is main's fix(spec): withdraw the ADR-0087 field-required-notnull-explicit conversion — required: true stops stamping storage.notNull #16890 text and this PR's comment additions sit in the numeric arm, not in that block. git merge-tree --write-tree origin/main refs/pr-review/16887 (main @ 854639b3, feat(engine)!: findOne, update and delete declare what they answer, and their hook seams are guarded (#16231) #16783 landed) → clean; API mergeable: true / clean.
  • Ownership: card #16318 is pm:queue, assignee empty, needs:contract-review on both carriers; the last Claim: on the card is the consolidated seat's takeover (5594621006, 02:00:11Z), whose dev was killed by a 429 at ~03:29Z (5597041636) after pushing 034856799a with no os-dev-report. The sibling branch claude/issue-16318-patch-dup-01ADLdAs @ 5339717b6d was NOT reviewed here; its report was read as claims only.
  • Dev report (5595029333, the latest on the card) — open questions answered. Q1 (which line lands): A — keep 034856799a; it is the head under review, CI-green, and F1–F6 are discharged on it as verified above. Take from 5339717b6d only the five comment fossils (Finding F3); do NOT take the four extra changeset packages (not owed); the F2 wording upgrade is already on the head (:43-44 "with no error"). Q2 (double dispatch or resumed session): a deliberate takeover claimed on the card5594621006 is an explicit Claim: posted before the domain:engine seat's ~02:1xZ re-dispatch, and the head commit's trailer names that claim's session; per CLAUDE.md the newest Claim: decides, so the stand-down in 5595053286 was the correct outcome. deviations: the report declares none (no key); its out_of_scope_findings on the generated-mdx hazard is confirmed on the head; its "both servers available in this container" is an environment claim I did not need and did not verify.

Findings

  • F1 — non-blocking. packages/spec/src/data/numeric-column-representation.ts:100-106 still states the residual bound the prior review's F5 corrected ("Magnitudes below 1e-30 round to zero"), while the changeset :47-52 states "30 fractional digits are kept … loss begins around |x| < 1e-13". The docblock says of itself "stated here so no reader has to rediscover it", and sql-driver.ts:16487 and generate.ts both tell readers the spec module "carries the residual bound" — so the canonical home now disagrees with the shipped changelog. One sentence.
  • F2 — non-blocking. Changeset self-references: :39 "Three consequences" above four bullets (:41, :47, :53, :60); the ADR-0087 marker :70 says "bullet 3 IS a prescription", but bullet 3 (:53) is the read-seam bullet and the prescription is bullet 4 (:60). The marker is the auditable artifact the gate prints verbatim; a wrong pointer defeats its purpose. Fix with F1 in one commit.
  • F3 — non-blocking. The same summary-is-table.float sentence corrected at sortability.zod.ts:64 still stands in five comment sites the PR does not touch: packages/metadata-protocol/src/protocol.ts:3087 (docblock table) and :9400; packages/lint/src/validate-sortable-fields.ts:70; packages/lint/src/validate-sortable-fields.test.ts:27, :105; packages/objectql/src/query-expression-conformance.test.ts:92, :602; packages/services/service-analytics/src/__tests__/measure-result-type.test.ts:189, :666. Code comments, no published page (verified by grep of content/docs on the head) — the [finding] Seven out-of-package comments still say formatOutput's audit and datetime repairs sit inside if (this.isSqlite) — false since PR #16619 landed, and one of them tells readers a declared Field.datetime is NOT protected on Postgres/MySQL #16728/[finding] Eleven more out-of-package comments still gate formatOutput's timestamp repairs on if (this.isSqlite) — the same drift as #16728, invisible to its normalizeSqliteDatetimeOutput census #16818 class, not the F1 class. Sweep here (the sibling 5339717b6d carries it) or file a follow-up; either is acceptable.
  • F4 — non-blocking. The PR body is behind the head: it pins every reading to c0d4ed6f9f, lists an 11-file surface (the head has 16) and omits the live-dialect cell, sortability.zod.ts/.mdx, types.mdx and measure-result-type.ts; no os-dev-report exists for the head's patch round. The body is what the maintainer reads at merge time; it must be brought to the head by whoever re-claims.
  • F5 — record. Evidence that the live cells executed on MySQL is step-level, not log-line-level: the Temporal job on the head (102317539991) completed success with step 12 "Run driver-sql suite against both live servers" success, under the flag and URLs cited above, and the harness makes a missing server red. The per-file log line was not retrievable from this seat (the REST log redirect targets productionresultssa*.blob.core.windows.net, which the proxy answers 403 CONNECT; the MCP log tail is capped at ~600 KB and begins inside step 14). The head's own commit message says the MySQL half was run only in CI; the sibling line's local MySQL 8.0.46 reading (4.5 → 5, 4.4 → 4, SHOW WARNINGS empty) agrees with the pinned expectation.
  • F6 — observation. No .skip/.only/.todo in the diff; no throw introduces a registered error code; sql-driver-16318-numeric-representation.test.ts (SQLite) and the spec pin are unchanged since c0d4ed6f9 and still derive from spec constants. Per-type SQLite consequences are stated in the spec module, the driver arm (sql-driver.ts:16494-16512) and types.mdx:1245-1251.

Maintainer-only merge: yes — unchanged from the prior review's reading (feat!, Clause-②: yes, cross-lane, decides what every new table gets for money/percent/rating on PG and MySQL under a 「继续决策」 ruling). Per 5586839197, on this PASS the card goes pm:awaiting-maintainer and the PR stays draft; F1/F2 are a one-commit tidy the maintainer may want before reading, and do not gate.

CI at read time

Head 034856799a4107a5c965be47f99ae2cf4d023877: 38 check runs, 34 latest-per-name — 30 success, 4 skipped (Auto Label, Check PR Size, Console Pin Gate, Packed-tarball smoke (opt-in)), 0 failed, 0 in progress; every run's head_sha is the head. Green includes Build Core, Test Core (+6 shards), Temporal Conformance (live PG + MySQL), Type Check ×5, Check Changeset, Lint & Repo Gates, Governed Surface Queue Guard, Spec property liveness, Dogfood ×5, both single-writer/same-issue guards. PR: draft, mergeable_state: clean, labels documentation, size/xl, tests, tooling, protocol:data, needs:contract-review.

Implemented-by: branch claude/issue-16318-numeric-column-representation-table
Reviewed-by: director seat summon #20 (isolated fable subagent, transcript-verified before adoption)

{"pr":16887,"head":"034856799a4107a5c965be47f99ae2cf4d023877","verdict":"PASS WITH FINDINGS","blocking":[],"clause2":"yes","semver_ok":true,"governed":false,"ci":"38 runs / 34 latest-per-name: 30 success, 4 skipped (Auto Label, Check PR Size, Console Pin Gate, Packed-tarball smoke opt-in), 0 failed, 0 pending; mergeable clean"}


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants